replace MethodInfoKey Pair alias with struct to avoid type piracy#149
replace MethodInfoKey Pair alias with struct to avoid type piracy#149
MethodInfoKey Pair alias with struct to avoid type piracy#149Conversation
Changes `MethodInfoKey` from a `Pair` type alias to a proper struct,
avoiding type piracy that would be introduced by defining
`Pair{...}(::Method)` conversion methods. The struct implements
`Base.iterate` to maintain compatibility with destructuring patterns.
This is a breaking change as it changes the representation of method
info keys, which packages like LCU and Revise depend heavily on. So we
unfotunately need to bump the major version from 2 to 3.
As an alternative, removing `Pair{...}(::Method)` would also be
possible, but that would also break LCU and Revise, so I'm taking this
approach which appears to be a better and moer complete fix.
|
If there are no objections, I would like to proceed with this PR in the direction of releasing a new major version (this is needed for JETLS to use Revise for development, which will use a vendored CodeTracking implementation internally and would cause precompilatio warning without this change: aviatesk/JETLS.jl#314). @timholy Afraid to ping you, but please let me know if you have any concerns. If there's no response, I will take responsibility for moving this PR forward while updating the Revise ecosystem myself. |
|
Sorry I didn't notice this. It was fine to merge this. Feel free to ping me on slack any time. (There are plenty of times where my other responsibilities make it unrealistic to spend the ~40min per day it requires to stay on top of my notifications.) |
3 participants
Changes
MethodInfoKeyfrom aPairtype alias to a proper struct, avoiding type piracy that would be introduced by definingPair{...}(::Method)conversion methods. The struct implementsBase.iterateto maintain compatibility with destructuring patterns.This is a breaking change as it changes the representation of method info keys, which packages like LCU and Revise depend heavily on. So we unfotunately need to bump the major version from 2 to 3.
As an alternative, removing
Pair{...}(::Method)would also be possible, but that would also break LCU and Revise, so I'm taking this approach which appears to be a better and moer complete fix.